DataStoreStorage replaces the deprecated SharedPreferencesStorage #259
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
library/src/main/java/io/fusionauth/mobilesdk/storage/DataStoreStorage.kt
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Pull request overview
This PR deprecates SharedPreferencesStorage in favor of the new DataStoreStorage implementation for token persistence. The change aligns with modern Android development practices by adopting Jetpack DataStore.
Key Changes:
- Introduced
DataStoreStorageclass using Jetpack DataStore for persistence - Deprecated
SharedPreferencesStoragewith a warning-level annotation - Updated sample app activities to use the new
DataStoreStorage
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| SharedPreferencesStorage.kt | Added deprecation annotation directing users to DataStoreStorage |
| DataStoreStorage.kt | New storage implementation using Jetpack DataStore with synchronous API wrapping coroutines |
| library/build.gradle.kts | Added DataStore preferences dependency (version 1.2.0) |
| TokenActivity.kt | Replaced SharedPreferencesStorage with DataStoreStorage in initialization |
| LoginActivity.kt | Replaced SharedPreferencesStorage with DataStoreStorage in initialization |
| README.md | Updated documentation to reference DataStoreStorage instead of SharedPreferencesStorage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return runBlocking { | ||
| context.dataStore.data.first()[prefKey] | ||
| } |
There was a problem hiding this comment.
Using runBlocking on the main thread for storage operations can cause UI freezes and ANR (Application Not Responding) errors. Consider making the Storage interface suspend-based or providing async alternatives, as DataStore is designed to be used with coroutines asynchronously.
| runBlocking { | ||
| context.dataStore.edit { settings -> | ||
| settings[prefKey] = content.toString() | ||
| } | ||
| } |
There was a problem hiding this comment.
Using runBlocking on the main thread for storage operations can cause UI freezes and ANR errors. Consider making the Storage interface suspend-based or providing async alternatives.
| runBlocking { | ||
| context.dataStore.edit { settings -> | ||
| settings.remove(prefKey) | ||
| } | ||
| } |
There was a problem hiding this comment.
Using runBlocking on the main thread for storage operations can cause UI freezes and ANR errors. Consider making the Storage interface suspend-based or providing async alternatives.
| */ | ||
| @Deprecated( | ||
| message = "Use DataStoreStorage instead", | ||
| replaceWith = ReplaceWith("DataStoreStorage"), |
There was a problem hiding this comment.
The ReplaceWith should include the full qualified name or import statement. Use ReplaceWith(\"DataStoreStorage\", \"io.fusionauth.mobilesdk.storage.DataStoreStorage\") to ensure IDEs can properly auto-import when applying the replacement.
| replaceWith = ReplaceWith("DataStoreStorage"), | |
| replaceWith = ReplaceWith("DataStoreStorage", "io.fusionauth.mobilesdk.storage.DataStoreStorage"), |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| AuthorizationManager.clearState() | ||
| lifecycleScope.launch { | ||
| AuthorizationManager.clearState() | ||
| } |
There was a problem hiding this comment.
The signOut() method finishes with starting a new activity, but the clearState() operation is async and may not complete before the activity transition. Consider awaiting the clearState() call completion before starting the new activity.
| */ | ||
| override suspend fun get(key: String): String? { | ||
| val prefKey = stringPreferencesKey(key) | ||
| return context.dataStore.data.first()[prefKey] |
There was a problem hiding this comment.
Using .first() on DataStore.data will block until the first value is available. For better performance and error handling, consider using .map { it[prefKey] }.first() which can provide more granular control over the data flow.
| return context.dataStore.data.first()[prefKey] | |
| return context.dataStore.data.map { it[prefKey] }.first() |
DataStoreStorage replaces the deprecated SharedPreferencesStorage as a way to store tokens.